Skip to content

Show discoverLayers errors in Problems#164

Merged
jthuangarm merged 3 commits into
Open-CMSIS-Pack:mainfrom
jthuangarm:discoverlayer
Apr 8, 2026
Merged

Show discoverLayers errors in Problems#164
jthuangarm merged 3 commits into
Open-CMSIS-Pack:mainfrom
jthuangarm:discoverlayer

Conversation

@jthuangarm
Copy link
Copy Markdown
Contributor

@jthuangarm jthuangarm commented Apr 8, 2026

Fixes

Changes

Improve error handling and reporting in the solution conversion process, specifically focusing on failures during the layer discovery step.

Error handling and reporting improvements:

  • Updated the checkDiscoverLayers method in SolutionConverterImpl to return both a success flag and an array of error messages, ensuring that any failure messages from layer discovery are captured and formatted for user output. [1] [2]
  • Modified the solution conversion logic to append error messages from discoverLayers to the toolsOutputMessages array.

Testing enhancements:

  • Added a new test case in solution-converter.test.ts to verify that a failure in discoverLayers results in an appropriate error message being reported through toolsOutputMessages.

Screenshots

image

Checklist

  • 🤖 This change is covered by unit tests (if applicable).
  • 🤹 Manual testing has been performed (if necessary).
  • 🛡️ Security impacts have been considered (if relevant).
  • 📖 Documentation updates are complete (if required).
  • 🧠 Third-party dependencies and TPIP updated (if required).

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves how the extension surfaces DiscoverLayers failures during solution conversion by propagating discovery errors into toolsOutputMessages, enabling them to appear in VS Code’s Problems view (addressing #119).

Changes:

  • Update SolutionConverterImpl.checkDiscoverLayers to return both a success flag and formatted tool error output.
  • Append DiscoverLayers failure messages into toolsOutputMessages during conversion.
  • Add a unit test validating that DiscoverLayers failures are reported via toolsOutputMessages.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/solutions/solution-converter.ts Capture DiscoverLayers failure messages and include them in conversion output (toolsOutputMessages).
src/solutions/solution-converter.test.ts Add coverage ensuring DiscoverLayers failures propagate to toolsOutputMessages.
Comments suppressed due to low confidence (1)

src/solutions/solution-converter.ts:257

  • checkDiscoverLayers always appends "Discover Layers..." to the output channel, but on failure it never prints the failure reason (result.message) to the output channel. This makes the Output view unhelpful when discovery fails. Consider appending the error details (when !result.success) to the output channel as well, similar to printErrorsWarnings.
        const outputChannel = this.outputChannelProvider.getOrCreate(manifest.CMSIS_SOLUTION_OUTPUT_CHANNEL);
        this.solutionManager.getCsolution()?.setVariablesConfigurations(undefined);
        // rpc method: DiscoverLayers
        outputChannel.append('Discover Layers... ');
        const result = await this.cmsisToolboxManager.runCsolutionRpc(

Comment thread src/solutions/solution-converter.ts Outdated
@jthuangarm jthuangarm marked this pull request as ready for review April 8, 2026 09:30
@jthuangarm jthuangarm requested a review from brondani April 8, 2026 09:30
Copy link
Copy Markdown
Collaborator

@brondani brondani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jthuangarm jthuangarm merged commit 2fe9800 into Open-CMSIS-Pack:main Apr 8, 2026
10 checks passed
@jthuangarm jthuangarm deleted the discoverlayer branch April 8, 2026 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants